projects / openwrt / openwrt.git / blob
? search:


c7b4ea58dcb94ffc0fba23aa22b4b1cfb447ff56
[openwrt/openwrt.git] /
1 From 447dd46f95014eb4ea94f6164963bf23ce05b927 Mon Sep 17 00:00:00 2001
2 From: Jonas Jelonek <jelonek.jonas@gmail.com>
3 Date: Sat, 27 Sep 2025 10:19:28 +0000
4 Subject: [PATCH] i2c: rtl9300: separate xfer configuration and execution
5
6 So far, the rtl9300_i2c_smbus_xfer code is quite a mess with function
7 calls distributed over the whole function setting different values in
8 different cases. Calls to rtl9300_i2c_config_xfer and
9 rtl9300_i2c_reg_addr_set are used in every case-block with varying
10 values whose meaning is not instantly obvious. In some cases, there are
11 additional calls within these case-blocks doing more things.
12
13 This is in general a bad design and especially really bad for
14 readability and maintainability because it distributes changes or
15 issues to multiple locations due to the same function being called with
16 different hardcoded values in different places.
17
18 To have a good structure, setting different parameters based on the
19 desired operation should not be interleaved with applying these
20 parameters to the hardware registers. Or in different words, the
21 parameter site should be mixed with the call site.
22
23 Thus, separate configuration and execution of an SMBus xfer within
24 rtl9300_i2c_smbus_xfer to improve readability and maintainability. Add a
25 new 'struct rtl9300_i2c_xfer' to carry the required parameters for an
26 xfer which are configured based on the input parameters within a single
27 switch-case block, without having any function calls within this block.
28
29 The function calls to actually apply these values to the hardware
30 registers then appear below in a single place and just operate on the
31 passed instance of 'struct rtl9300_i2c_xfer'. These are
32 'rtl9300_i2c_prepare_xfer' which combines applying all parameters of the
33 xfer to the corresponding register, and 'rtl9300_i2c_do_xfer' which
34 actually executes the xfer and does post-processing if needed.
35
36 Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
37 Tested-by: Sven Eckelmann <sven@narfation.org>
38 Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
39 Tested-by: Chris Packham <chris.packham@alliedtelesis.co.nz> # On RTL9302C based board
40 Tested-by: Markus Stockhausen <markus.stockhausen@gmx.de>
41 Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
42 Link: https://lore.kernel.org/r/20250927101931.71575-7-jelonek.jonas@gmail.com
43
44 diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
45 index 9e3517b09b3d..fb3ebbd46a18 100644
46 --- a/drivers/i2c/busses/i2c-rtl9300.c
47 +++ b/drivers/i2c/busses/i2c-rtl9300.c
48 @@ -8,6 +8,7 @@
49 #include <linux/mutex.h>
50 #include <linux/platform_device.h>
51 #include <linux/regmap.h>
52 +#include <linux/unaligned.h>
53
54 enum rtl9300_bus_freq {
55 RTL9300_I2C_STD_FREQ,
56 @@ -71,6 +72,22 @@ struct rtl9300_i2c {
57 struct mutex lock;
58 };
59
60 +enum rtl9300_i2c_xfer_type {
61 + RTL9300_I2C_XFER_BYTE,
62 + RTL9300_I2C_XFER_WORD,
63 + RTL9300_I2C_XFER_BLOCK,
64 +};
65 +
66 +struct rtl9300_i2c_xfer {
67 + enum rtl9300_i2c_xfer_type type;
68 + u16 dev_addr;
69 + u8 reg_addr;
70 + u8 reg_addr_len;
71 + u8 *data;
72 + u8 data_len;
73 + bool write;
74 +};
75 +
76 #define RTL9300_I2C_MST_CTRL1 0x0
77 #define RTL9300_I2C_MST_CTRL2 0x4
78 #define RTL9300_I2C_MST_DATA_WORD0 0x8
79 @@ -95,45 +112,37 @@ static int rtl9300_i2c_select_scl(struct rtl9300_i2c *i2c, u8 scl)
80 return regmap_field_write(i2c->fields[F_SCL_SEL], 1);
81 }
82
83 -static int rtl9300_i2c_config_io(struct rtl9300_i2c *i2c, struct rtl9300_i2c_chan *chan)
84 +static int rtl9300_i2c_config_chan(struct rtl9300_i2c *i2c, struct rtl9300_i2c_chan *chan)
85 {
86 struct rtl9300_i2c_drv_data *drv_data;
87 int ret;
88
89 - drv_data = (struct rtl9300_i2c_drv_data *)device_get_match_data(i2c->dev);
90 + if (i2c->sda_num == chan->sda_num)
91 + return 0;
92
93 - ret = regmap_field_update_bits(i2c->fields[F_SDA_SEL], BIT(chan->sda_num),
94 - BIT(chan->sda_num));
95 + ret = regmap_field_write(i2c->fields[F_SCL_FREQ], chan->bus_freq);
96 if (ret)
97 return ret;
98
99 - ret = regmap_field_write(i2c->fields[F_SDA_OUT_SEL], chan->sda_num);
100 + drv_data = (struct rtl9300_i2c_drv_data *)device_get_match_data(i2c->dev);
101 + ret = drv_data->select_scl(i2c, 0);
102 if (ret)
103 return ret;
104
105 - ret = regmap_field_write(i2c->fields[F_SCL_FREQ], chan->bus_freq);
106 + ret = regmap_field_update_bits(i2c->fields[F_SDA_SEL], BIT(chan->sda_num),
107 + BIT(chan->sda_num));
108 if (ret)
109 return ret;
110
111 - return drv_data->select_scl(i2c, 0);
112 -}
113 -
114 -static int rtl9300_i2c_config_xfer(struct rtl9300_i2c *i2c, struct rtl9300_i2c_chan *chan,
115 - u16 addr, u16 len)
116 -{
117 - int ret;
118 -
119 - if (len < 1 || len > 16)
120 - return -EINVAL;
121 -
122 - ret = regmap_field_write(i2c->fields[F_DEV_ADDR], addr);
123 + ret = regmap_field_write(i2c->fields[F_SDA_OUT_SEL], chan->sda_num);
124 if (ret)
125 return ret;
126
127 - return regmap_field_write(i2c->fields[F_DATA_WIDTH], (len - 1) & 0xf);
128 + i2c->sda_num = chan->sda_num;
129 + return 0;
130 }
131
132 -static int rtl9300_i2c_read(struct rtl9300_i2c *i2c, u8 *buf, int len)
133 +static int rtl9300_i2c_read(struct rtl9300_i2c *i2c, u8 *buf, u8 len)
134 {
135 u32 vals[4] = {};
136 int i, ret;
137 @@ -153,7 +162,7 @@ static int rtl9300_i2c_read(struct rtl9300_i2c *i2c, u8 *buf, int len)
138 return 0;
139 }
140
141 -static int rtl9300_i2c_write(struct rtl9300_i2c *i2c, u8 *buf, int len)
142 +static int rtl9300_i2c_write(struct rtl9300_i2c *i2c, u8 *buf, u8 len)
143 {
144 u32 vals[4] = {};
145 int i;
146 @@ -176,16 +185,51 @@ static int rtl9300_i2c_writel(struct rtl9300_i2c *i2c, u32 data)
147 return regmap_write(i2c->regmap, i2c->data_reg, data);
148 }
149
150 -static int rtl9300_i2c_execute_xfer(struct rtl9300_i2c *i2c, char read_write,
151 - int size, union i2c_smbus_data *data, int len)
152 +static int rtl9300_i2c_prepare_xfer(struct rtl9300_i2c *i2c, struct rtl9300_i2c_xfer *xfer)
153 {
154 - u32 val;
155 int ret;
156
157 - ret = regmap_field_write(i2c->fields[F_RWOP], read_write == I2C_SMBUS_WRITE);
158 + if (xfer->data_len < 1 || xfer->data_len > 16)
159 + return -EINVAL;
160 +
161 + ret = regmap_field_write(i2c->fields[F_DEV_ADDR], xfer->dev_addr);
162 + if (ret)
163 + return ret;
164 +
165 + ret = rtl9300_i2c_reg_addr_set(i2c, xfer->reg_addr, xfer->reg_addr_len);
166 + if (ret)
167 + return ret;
168 +
169 + ret = regmap_field_write(i2c->fields[F_RWOP], xfer->write);
170 + if (ret)
171 + return ret;
172 +
173 + ret = regmap_field_write(i2c->fields[F_DATA_WIDTH], (xfer->data_len - 1) & 0xf);
174 if (ret)
175 return ret;
176
177 + if (xfer->write) {
178 + switch (xfer->type) {
179 + case RTL9300_I2C_XFER_BYTE:
180 + ret = rtl9300_i2c_writel(i2c, *xfer->data);
181 + break;
182 + case RTL9300_I2C_XFER_WORD:
183 + ret = rtl9300_i2c_writel(i2c, get_unaligned((const u16 *)xfer->data));
184 + break;
185 + default:
186 + ret = rtl9300_i2c_write(i2c, xfer->data, xfer->data_len);
187 + break;
188 + }
189 + }
190 +
191 + return ret;
192 +}
193 +
194 +static int rtl9300_i2c_do_xfer(struct rtl9300_i2c *i2c, struct rtl9300_i2c_xfer *xfer)
195 +{
196 + u32 val;
197 + int ret;
198 +
199 ret = regmap_field_write(i2c->fields[F_I2C_TRIG], 1);
200 if (ret)
201 return ret;
202 @@ -200,28 +244,24 @@ static int rtl9300_i2c_execute_xfer(struct rtl9300_i2c *i2c, char read_write,
203 if (val)
204 return -EIO;
205
206 - if (read_write == I2C_SMBUS_READ) {
207 - switch (size) {
208 - case I2C_SMBUS_BYTE:
209 - case I2C_SMBUS_BYTE_DATA:
210 + if (!xfer->write) {
211 + switch (xfer->type) {
212 + case RTL9300_I2C_XFER_BYTE:
213 ret = regmap_read(i2c->regmap, i2c->data_reg, &val);
214 if (ret)
215 return ret;
216 - data->byte = val & 0xff;
217 +
218 + *xfer->data = val & 0xff;
219 break;
220 - case I2C_SMBUS_WORD_DATA:
221 + case RTL9300_I2C_XFER_WORD:
222 ret = regmap_read(i2c->regmap, i2c->data_reg, &val);
223 if (ret)
224 return ret;
225 - data->word = val & 0xffff;
226 - break;
227 - case I2C_SMBUS_I2C_BLOCK_DATA:
228 - ret = rtl9300_i2c_read(i2c, &data->block[1], len);
229 - if (ret)
230 - return ret;
231 +
232 + put_unaligned(val & 0xffff, (u16*)xfer->data);
233 break;
234 default:
235 - ret = rtl9300_i2c_read(i2c, &data->block[0], len);
236 + ret = rtl9300_i2c_read(i2c, xfer->data, xfer->data_len);
237 if (ret)
238 return ret;
239 break;
240 @@ -237,108 +277,62 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
241 {
242 struct rtl9300_i2c_chan *chan = i2c_get_adapdata(adap);
243 struct rtl9300_i2c *i2c = chan->i2c;
244 - int len = 0, ret;
245 + struct rtl9300_i2c_xfer xfer = {0};
246 + int ret;
247 +
248 + if (addr > 0x7f)
249 + return -EINVAL;
250
251 mutex_lock(&i2c->lock);
252 - if (chan->sda_num != i2c->sda_num) {
253 - ret = rtl9300_i2c_config_io(i2c, chan);
254 - if (ret)
255 - goto out_unlock;
256 - i2c->sda_num = chan->sda_num;
257 - }
258 +
259 + ret = rtl9300_i2c_config_chan(i2c, chan);
260 + if (ret)
261 + goto out_unlock;
262 +
263 + xfer.dev_addr = addr & 0x7f;
264 + xfer.write = (read_write == I2C_SMBUS_WRITE);
265 + xfer.reg_addr = command;
266 + xfer.reg_addr_len = 1;
267
268 switch (size) {
269 case I2C_SMBUS_BYTE:
270 - if (read_write == I2C_SMBUS_WRITE) {
271 - ret = rtl9300_i2c_config_xfer(i2c, chan, addr, 0);
272 - if (ret)
273 - goto out_unlock;
274 - ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
275 - if (ret)
276 - goto out_unlock;
277 - } else {
278 - ret = rtl9300_i2c_config_xfer(i2c, chan, addr, 1);
279 - if (ret)
280 - goto out_unlock;
281 - ret = rtl9300_i2c_reg_addr_set(i2c, 0, 0);
282 - if (ret)
283 - goto out_unlock;
284 - }
285 + xfer.data = (read_write == I2C_SMBUS_READ) ? &data->byte : &command;
286 + xfer.data_len = 1;
287 + xfer.reg_addr = 0;
288 + xfer.reg_addr_len = 0;
289 + xfer.type = RTL9300_I2C_XFER_BYTE;
290 break;
291 -
292 case I2C_SMBUS_BYTE_DATA:
293 - ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
294 - if (ret)
295 - goto out_unlock;
296 - ret = rtl9300_i2c_config_xfer(i2c, chan, addr, 1);
297 - if (ret)
298 - goto out_unlock;
299 - if (read_write == I2C_SMBUS_WRITE) {
300 - ret = rtl9300_i2c_writel(i2c, data->byte);
301 - if (ret)
302 - goto out_unlock;
303 - }
304 + xfer.data = &data->byte;
305 + xfer.data_len = 1;
306 + xfer.type = RTL9300_I2C_XFER_BYTE;
307 break;
308 -
309 case I2C_SMBUS_WORD_DATA:
310 - ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
311 - if (ret)
312 - goto out_unlock;
313 - ret = rtl9300_i2c_config_xfer(i2c, chan, addr, 2);
314 - if (ret)
315 - goto out_unlock;
316 - if (read_write == I2C_SMBUS_WRITE) {
317 - ret = rtl9300_i2c_writel(i2c, data->word);
318 - if (ret)
319 - goto out_unlock;
320 - }
321 + xfer.data = (u8 *)&data->word;
322 + xfer.data_len = 2;
323 + xfer.type = RTL9300_I2C_XFER_WORD;
324 break;
325 -
326 case I2C_SMBUS_BLOCK_DATA:
327 - ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
328 - if (ret)
329 - goto out_unlock;
330 - if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX) {
331 - ret = -EINVAL;
332 - goto out_unlock;
333 - }
334 - ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0] + 1);
335 - if (ret)
336 - goto out_unlock;
337 - if (read_write == I2C_SMBUS_WRITE) {
338 - ret = rtl9300_i2c_write(i2c, &data->block[0], data->block[0] + 1);
339 - if (ret)
340 - goto out_unlock;
341 - }
342 - len = data->block[0] + 1;
343 + xfer.data = &data->block[0];
344 + xfer.data_len = data->block[0] + 1;
345 + xfer.type = RTL9300_I2C_XFER_BLOCK;
346 break;
347 -
348 case I2C_SMBUS_I2C_BLOCK_DATA:
349 - ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
350 - if (ret)
351 - goto out_unlock;
352 - if (data->block[0] < 1 || data->block[0] > I2C_SMBUS_BLOCK_MAX) {
353 - ret = -EINVAL;
354 - goto out_unlock;
355 - }
356 - ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0]);
357 - if (ret)
358 - goto out_unlock;
359 - if (read_write == I2C_SMBUS_WRITE) {
360 - ret = rtl9300_i2c_write(i2c, &data->block[1], data->block[0]);
361 - if (ret)
362 - goto out_unlock;
363 - }
364 - len = data->block[0];
365 + xfer.data = &data->block[1];
366 + xfer.data_len = data->block[0];
367 + xfer.type = RTL9300_I2C_XFER_BLOCK;
368 break;
369 -
370 default:
371 dev_err(&adap->dev, "Unsupported transaction %d\n", size);
372 ret = -EOPNOTSUPP;
373 goto out_unlock;
374 }
375
376 - ret = rtl9300_i2c_execute_xfer(i2c, read_write, size, data, len);
377 + ret = rtl9300_i2c_prepare_xfer(i2c, &xfer);
378 + if (ret)
379 + goto out_unlock;
380 +
381 + ret = rtl9300_i2c_do_xfer(i2c, &xfer);
382
383 out_unlock:
384 mutex_unlock(&i2c->lock);
385 --
386 2.48.1
387
OpenWrt Source Repository